Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] cetmix_tower_server: implement UI updates #185

Open
wants to merge 9 commits into
base: 14.0-dev
Choose a base branch
from

Conversation

tendil
Copy link
Collaborator

@tendil tendil commented Jan 17, 2025

This commit implements the following UI updates:

  1. Sort Flight Plan Log and Command Log by "Started" (newest first).

    • Added default_order="start_date desc, id desc" in tree views for both logs.
  2. Show related Flight Plan Log line in the Command Log form.

    • Added related fields in the Command Log model and updated the form view.
  3. Update the "Code" field in the Flight Plan Line form.

    • Implemented a computed field command_code to display:
    • File template code for the action "Upload File from Template".
    • Related flight plan lines for the action "Execute Flight Plan".
  4. Display the number of servers in the Server Template Kanban view.

    • Used the server_count computed field to show the count as a numeric value in Kanban cards.
  5. Add a pre-defined "Flight Plan" filter to the Command search view.

    • Added a filter for the action='plan' in the search view of the Command model.

Task ID: 4263

Summary by CodeRabbit

Release Notes

  • New Features

    • Added server count display in kanban view.
    • Introduced new filter for flight plan commands.
    • Added archiving functionality with "Archive" and "Unarchive" buttons for file templates.
    • New fields added to enhance relationships and visibility in various models.
  • Improvements

    • Updated default sorting for command logs and plan logs to prioritize start date.
    • Enhanced command line view with conditional field visibility.
    • Modified command code and added plan line relationship in tower plan line model.
    • Introduced a new field for tracking active status of file templates.
  • UI Changes

    • Refined server template kanban view layout.
    • Updated form view for command line preview.
    • Enhanced form view for file templates with new buttons and filters.
    • Added visibility control for new fields in various views.

@tendil tendil requested a review from ivs-cetmix January 17, 2025 01:51
Copy link

coderabbitai bot commented Jan 17, 2025

Walkthrough

The pull request introduces modifications to several files in the cetmix_tower_server directory. The changes primarily focus on updating the CxTowerPlanLine model by adding new fields plan_line_ids and flight_plan_id, along with updating the command_code field. Various view XML files have also been updated to enhance field visibility, sorting order, and search functionality across models such as command logs, commands, plan logs, and server templates, including the addition of archiving capabilities for file templates.

Changes

File Change Summary
models/cx_tower_plan_line.py - Added plan_line_ids one-to-many relationship field
- Added flight_plan_id many-to-one relationship field
- Added file_template_id many-to-one relationship field
- Updated command_code field with comodel_name and string attributes
views/cx_tower_command_log_view.xml - Added plan_log_id field with visibility control
- Updated tree view default sorting to start_date desc, id desc
views/cx_tower_command_view.xml - Added "Run Flight Plan" filter in search view
views/cx_tower_plan_log_view.xml - Updated tree view default sorting to start_date desc, id desc
views/cx_tower_server_template_view.xml - Added server_count field
- Removed reference field
- Added conditional display for server_count
views/cx_tower_plan_line_view.xml - Added action field
- Updated command_code, use_sudo, and path fields with conditional visibility
- Added group for flight_plan_id and plan_line_ids with conditional visibility
models/cx_tower_file_template.py - Added active Boolean field with default value True
views/cx_tower_file_template_view.xml - Added "Archive" and "Unarchive" buttons with visibility control
- Added "Archived" filter in search view
- Added invisible active field in form view

Possibly related PRs

Suggested labels

approved

Suggested reviewers

  • GabbasovDinar
  • Aldeigja
  • ivs-cetmix

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a55a1b7 and 7126c42.

📒 Files selected for processing (1)
  • cetmix_tower_server/models/cx_tower_plan_line.py (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • cetmix_tower_server/models/cx_tower_plan_line.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test with OCB

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
cetmix_tower_server/models/cx_tower_plan_line.py (1)

95-111: Enhance code quality with constants and improved documentation.

Consider the following improvements:

  1. Define constants for action types and default messages
  2. Enhance docstring with parameter and return type documentation
+ # Constants
+ ACTION_FILE_TEMPLATE = "file_using_template"
+ ACTION_PLAN = "plan"
+ DEFAULT_TEMPLATE_MSG = "No template code available"
+ DEFAULT_PLAN_MSG = "No related lines available"
+ DEFAULT_PREVIEW_MSG = "No preview available"

 @api.depends("command_id", "action")
 def _compute_command_code(self):
     """
     Compute the preview of the command based on its action.
+
+    Computes a preview text based on the command's action type:
+    - For file_using_template: Shows the template code
+    - For plan: Shows a list of flight plan line names
+    - For other actions: Shows a default message
+
+    Dependencies:
+        - command_id: The related command record
+        - action: The type of action to be performed
     """
     for line in self:
-        if line.action == "file_using_template":
+        if line.action == ACTION_FILE_TEMPLATE:
             line.command_code = (
-                line.command_id.code or "No template code available"
+                line.command_id.code or DEFAULT_TEMPLATE_MSG
             )
-        elif line.action == "plan":
+        elif line.action == ACTION_PLAN:
             flight_plan_lines = line.command_id.flight_plan_id.line_ids
             preview_lines = [line.name for line in flight_plan_lines]
-            line.command_code = "\n".join(preview_lines) if preview_lines else "No related lines available"
+            line.command_code = "\n".join(preview_lines) if preview_lines else DEFAULT_PLAN_MSG
         else:
-            line.command_code = "No preview available"
+            line.command_code = DEFAULT_PREVIEW_MSG
🧰 Tools
🪛 Ruff (0.8.2)

108-108: Line too long (111 > 88)

(E501)

🪛 GitHub Actions: pre-commit

[error] 108-108: Line too long (111 > 88 characters)

cetmix_tower_server/views/cx_tower_server_template_view.xml (1)

59-63: Consider UX enhancements for the server count display.

The implementation follows Odoo patterns and provides clear visibility of the count. Consider these UX improvements:

 <div attrs="{'invisible': [('server_count', '=', False)]}">
     <strong>Servers:</strong>
-    <span class="badge badge-primary">
+    <span class="badge badge-primary" 
+          t-att-title="'Number of servers using this template'">
         <t t-esc="record.server_count.value" />
     </span>
 </div>
🧰 Tools
🪛 GitHub Actions: pre-commit

[warning] File was modified by prettier formatting hook

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65a1c7a and 280b587.

📒 Files selected for processing (6)
  • cetmix_tower_server/models/cx_tower_command_log.py (1 hunks)
  • cetmix_tower_server/models/cx_tower_plan_line.py (2 hunks)
  • cetmix_tower_server/views/cx_tower_command_log_view.xml (2 hunks)
  • cetmix_tower_server/views/cx_tower_command_view.xml (1 hunks)
  • cetmix_tower_server/views/cx_tower_plan_log_view.xml (1 hunks)
  • cetmix_tower_server/views/cx_tower_server_template_view.xml (2 hunks)
🧰 Additional context used
🪛 GitHub Actions: pre-commit
cetmix_tower_server/views/cx_tower_server_template_view.xml

[warning] File was modified by prettier formatting hook

cetmix_tower_server/models/cx_tower_plan_line.py

[error] 108-108: Line too long (111 > 88 characters)

🪛 Ruff (0.8.2)
cetmix_tower_server/models/cx_tower_plan_line.py

108-108: Line too long (111 > 88)

(E501)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test with OCB
🔇 Additional comments (7)
cetmix_tower_server/models/cx_tower_plan_line.py (1)

47-51: LGTM! Field declaration follows Odoo conventions.

The change from a related field to a computed field is well-structured and aligns with the requirement to show dynamic content based on the action type.

cetmix_tower_server/models/cx_tower_command_log.py (1)

63-83: LGTM! Well-structured related fields implementation.

The new related fields for plan_log_id tracking are well-implemented with:

  • Proper field types matching the related model
  • Appropriate store=True for query optimization
  • Clear string labels for UI display
cetmix_tower_server/views/cx_tower_plan_log_view.xml (1)

84-84: LGTM! Sorting implementation matches requirements.

The default ordering by start_date desc, id desc aligns with the PR objective to sort Flight Plan Logs by "Started" date in descending order.

cetmix_tower_server/views/cx_tower_command_log_view.xml (2)

77-83: LGTM! Well-structured form view group for plan log tracking.

The new group of fields provides clear visibility of related Flight Plan Log details, matching the PR objective.


109-109: LGTM! Consistent sorting implementation.

The default ordering matches the Flight Plan Log view, maintaining consistency across the application.

cetmix_tower_server/views/cx_tower_command_view.xml (1)

253-257: LGTM! Search filter enhances command filtering capabilities.

The new filter for Run Flight Plan commands improves the search functionality by allowing specific filtering of flight plan related commands.

cetmix_tower_server/views/cx_tower_server_template_view.xml (1)

14-14: LGTM! Verify the computed field implementation.

The field declaration follows Odoo conventions and is properly positioned with related fields.

Run this script to verify the computed field implementation:

✅ Verification successful

✓ Computed field implementation verified and correct

The server_count field is properly implemented with a clean One2many relationship and follows Odoo best practices. The computation is lightweight and suitable for real-time display in views.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify server_count computed field implementation
# Expected: Find the compute method definition and dependencies

# Search for server_count compute method
ast-grep --pattern 'server_count = fields.$_($$$, compute="$_")'

# Search for potential dependencies in compute method
rg -A 10 "def _compute_server_count"

Length of output: 1293


Script:

#!/bin/bash
# Verify server_ids field definition and related performance settings
ast-grep --pattern 'server_ids = fields.$_($$$)'

# Check for potential performance impact by finding server count usage
rg "server_count" -A 2

Length of output: 5385

🧰 Tools
🪛 GitHub Actions: pre-commit

[warning] File was modified by prettier formatting hook

cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
This commit implements the following UI updates:

1. Sort Flight Plan Log and Command Log by "Started" (newest first).
   - Added `default_order="start_date desc, id desc"` in tree views for both logs.

2. Show related Flight Plan Log line in the Command Log form.
   - Added related fields in the Command Log model and updated the form view.

3. Update the "Code" field in the Flight Plan Line form.
   - Implemented a computed field `command_code` to display:
     - File template code for the action "Upload File from Template".
     - Related flight plan lines for the action "Execute Flight Plan".

4. Display the number of servers in the Server Template Kanban view.
   - Used the `server_count` computed field to show the count as a numeric value in Kanban cards.

5. Add a pre-defined "Flight Plan" filter to the Command search view.
   - Added a filter for the `action='plan'` in the search view of the Command model.

Task ID: 4263
@tendil tendil force-pushed the 14.0-t4263-cetmix_tower_server-ui-updates branch from 280b587 to 54fcb67 Compare January 17, 2025 01:54
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
cetmix_tower_server/models/cx_tower_plan_line.py (1)

102-103: Consider optimizing memory usage for large flight plans.

For large flight plans, building the preview list could be memory intensive. Consider limiting the number of lines shown.

-                flight_plan_lines = line.command_id.flight_plan_id.line_ids
-                preview_lines = [line.name for line in flight_plan_lines]
+                # Limit preview to first 10 lines for better performance
+                flight_plan_lines = line.command_id.flight_plan_id.line_ids[:10]
+                preview_lines = [line.name for line in flight_plan_lines]
+                if len(line.command_id.flight_plan_id.line_ids) > 10:
+                    preview_lines.append("... and more lines")
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 280b587 and 54fcb67.

📒 Files selected for processing (6)
  • cetmix_tower_server/models/cx_tower_command_log.py (1 hunks)
  • cetmix_tower_server/models/cx_tower_plan_line.py (2 hunks)
  • cetmix_tower_server/views/cx_tower_command_log_view.xml (2 hunks)
  • cetmix_tower_server/views/cx_tower_command_view.xml (1 hunks)
  • cetmix_tower_server/views/cx_tower_plan_log_view.xml (1 hunks)
  • cetmix_tower_server/views/cx_tower_server_template_view.xml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • cetmix_tower_server/views/cx_tower_command_view.xml
  • cetmix_tower_server/views/cx_tower_plan_log_view.xml
  • cetmix_tower_server/views/cx_tower_command_log_view.xml
  • cetmix_tower_server/models/cx_tower_command_log.py
  • cetmix_tower_server/views/cx_tower_server_template_view.xml
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test with OCB
🔇 Additional comments (1)
cetmix_tower_server/models/cx_tower_plan_line.py (1)

47-49: LGTM! Field declaration follows best practices.

The change from a related field to a computed field is well-structured and follows Odoo field declaration conventions.

cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54fcb67 and 5111416.

📒 Files selected for processing (3)
  • cetmix_tower_server/models/cx_tower_plan_line.py (2 hunks)
  • cetmix_tower_server/views/cx_tower_command_log_view.xml (2 hunks)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • cetmix_tower_server/views/cx_tower_command_log_view.xml
🔇 Additional comments (3)
cetmix_tower_server/views/cx_tower_plan_line_view.xml (1)

42-50: LGTM! Well-structured visibility rules.

The visibility rules for command_code and plan_line_ids fields are correctly implemented based on the action field value, following the feedback from the previous review.

cetmix_tower_server/models/cx_tower_plan_line.py (2)

110-120: LGTM! Well-implemented plan lines computation.

The _compute_plan_line_ids method correctly:

  • Handles the 'plan' action case
  • Includes proper null check for flight_plan
  • Sets False for non-plan actions

47-55: LGTM! Field definitions are clear and well-structured.

The computed fields are properly defined with correct parameters and string labels.

cetmix_tower_server/views/cx_tower_plan_line_view.xml Outdated Show resolved Hide resolved
cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
@tendil tendil force-pushed the 14.0-t4263-cetmix_tower_server-ui-updates branch from 5111416 to 8fec17f Compare January 17, 2025 14:43
@tendil tendil force-pushed the 14.0-t4263-cetmix_tower_server-ui-updates branch from 8fec17f to 76d5c23 Compare January 17, 2025 14:45
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
cetmix_tower_server/models/cx_tower_plan_line.py (1)

99-111: Simplify the command code computation logic.

The condition elif line.action != "plan" is redundant since we already handle the 'plan' action in the previous condition.

     @api.depends("command_id", "action")
     def _compute_command_code(self):
         """
         Compute the preview of the command based on its action.
         """
         for line in self:
             if line.action == "file_using_template":
                 line.command_code = line.command_id.code or "No template code available"
             elif line.action == "plan":
                 line.command_code = False
-            elif line.action != "plan":
+            else:
                 line.command_code = "No preview available"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5111416 and 76d5c23.

📒 Files selected for processing (3)
  • cetmix_tower_server/models/cx_tower_plan_line.py (2 hunks)
  • cetmix_tower_server/views/cx_tower_command_log_view.xml (2 hunks)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml
  • cetmix_tower_server/views/cx_tower_command_log_view.xml
🔇 Additional comments (2)
cetmix_tower_server/models/cx_tower_plan_line.py (2)

47-55: LGTM! Field declarations are well-structured.

The computed fields command_code and plan_line_ids are properly defined with appropriate field types and compute methods.


112-123: 🛠️ Refactor suggestion

Add validation for command_id in plan line computation.

The method could raise an AttributeError if command_id is False. Add validation to handle this case gracefully.

     @api.depends("command_id", "action")
     def _compute_plan_line_ids(self):
         """
         Compute the related plan lines if the action is "plan".
         """
         for line in self:
-            if line.action == "plan":
+            if not line.command_id:
+                line.plan_line_ids = False
+            elif line.action == "plan":
                 flight_plan = line.command_id.flight_plan_id
                 line.plan_line_ids = flight_plan.line_ids if flight_plan else []
             else:
                 line.plan_line_ids = False

Likely invalid or redundant comment.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
cetmix_tower_server/views/cx_tower_plan_line_view.xml (2)

41-41: Consider adding options and domain attributes to the action field.

The action field controls the visibility of other fields but lacks attributes to restrict its possible values. Consider adding:

  • widget="selection" for better UX
  • options="{'no_create': True, 'no_create_edit': True}" to prevent invalid values
  • A domain to restrict values to the expected set: ['ssh_command', 'python_code', 'plan']
-                                <field name="action" />
+                                <field 
+                                    name="action"
+                                    widget="selection"
+                                    options="{'no_create': True, 'no_create_edit': True}"
+                                />

42-49: Improve field visibility conditions and grouping.

The visibility conditions for command_code and plan_line_ids use different operators ('not in' vs '!=') for what appears to be mutually exclusive conditions. Consider:

  1. Using consistent operators for better maintainability
  2. Grouping related fields within a sub-group for better organization
-                                <field
-                                    name="command_code"
-                                    attrs="{'invisible': [('action', 'not in', ['ssh_command', 'python_code'])]}"
-                                />
-                                <field
-                                    name="plan_line_ids"
-                                    attrs="{'invisible': [('action', '!=', 'plan')]}"
-                                />
+                                <group>
+                                    <field
+                                        name="command_code"
+                                        attrs="{'invisible': [('action', 'not in', ['ssh_command', 'python_code'])],
+                                                'required': [('action', 'in', ['ssh_command', 'python_code'])]}"
+                                    />
+                                    <field
+                                        name="plan_line_ids"
+                                        attrs="{'invisible': [('action', 'not in', ['plan'])],
+                                                'required': [('action', 'in', ['plan'])]}"
+                                    />
+                                </group>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 76d5c23 and 905f390.

📒 Files selected for processing (3)
  • cetmix_tower_server/models/cx_tower_plan_line.py (1 hunks)
  • cetmix_tower_server/views/cx_tower_command_log_view.xml (2 hunks)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • cetmix_tower_server/views/cx_tower_command_log_view.xml
  • cetmix_tower_server/models/cx_tower_plan_line.py
🔇 Additional comments (1)
cetmix_tower_server/views/cx_tower_plan_line_view.xml (1)

41-49: Verify the action field values and field dependencies.

The view changes suggest that action can have values like 'ssh_command', 'python_code', and 'plan'. Let's verify:

  1. All possible values of the action field
  2. That these fields are properly defined in the model
  3. The relationships between these fields
✅ Verification successful

Field values and dependencies are correctly implemented

The action field values ('ssh_command', 'python_code', 'plan') and their relationships with command_code and plan_line_ids fields are properly defined and extensively tested in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify action field values and related field definitions

# Check the model definition for action field and its values
rg -A 10 "action = fields\." cetmix_tower_server/models/

# Check for any references to these specific action values
rg -e "ssh_command|python_code|plan" cetmix_tower_server/

# Check the model definitions for command_code and plan_line_ids
rg "command_code = fields\." cetmix_tower_server/models/
rg "plan_line_ids = fields\." cetmix_tower_server/models/

Length of output: 94595

)
plan_line_ids = fields.One2many(
comodel_name="cx.tower.plan.line",
compute="_compute_plan_line_ids",
inverse_name="command_id",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will allow to modify those lines directly which is not what we want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After analyzing and checking, I found that this field is redundant within the scope of this task. If this is not the case, please let me know

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
cetmix_tower_server/models/cx_tower_file_template.py (1)

26-26: Consider grouping fields together for better code organization.

The active field should be grouped with other fields for better readability and maintenance.

Move the field definition to be grouped with other fields:

-    active = fields.Boolean(default=True)
     file_name = fields.Char(
         help="Default full file name with file type for example: test.txt",
     )
+    active = fields.Boolean(default=True)
cetmix_tower_server/views/cx_tower_file_template_view.xml (1)

132-136: Consider enhancing the archive filter functionality.

While the implementation is correct, consider adding a default filter for active records and grouping them together.

Add default active filter:

+                <filter
+                    string="Active"
+                    name="active"
+                    domain="[('active', '=', True)]"
+                />
                 <filter
                     string="Archived"
                     name="archived"
                     domain="[('active', '=', False)]"
                 />
+                <separator/>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 905f390 and 4d472e3.

📒 Files selected for processing (4)
  • cetmix_tower_server/models/cx_tower_file_template.py (1 hunks)
  • cetmix_tower_server/models/cx_tower_plan_line.py (1 hunks)
  • cetmix_tower_server/views/cx_tower_file_template_view.xml (3 hunks)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml
  • cetmix_tower_server/models/cx_tower_plan_line.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test with OCB
🔇 Additional comments (3)
cetmix_tower_server/models/cx_tower_file_template.py (1)

26-26: Verify inheritance and data migration needs.

Please verify:

  1. That none of the inherited mixins (cx.tower.reference.mixin, cx.tower.key.mixin, cx.tower.template.mixin) already provide an active field to avoid conflicts.
  2. Whether existing records need a data migration script to set the active field.
✅ Verification successful

No conflicts or migration needed for the active field

  • None of the inherited mixins (cx.tower.reference.mixin, cx.tower.key.mixin, cx.tower.template.mixin) contain an active field, so there are no inheritance conflicts.
  • The default=True value ensures both new and existing records will work as expected without requiring data migration.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if active field exists in inherited mixins
echo "Checking for active field in inherited mixins..."
rg -A 5 "class.*Mixin.*:" | rg -A 5 "active.*=.*fields\."

# Check for existing records that might need migration
echo "Checking if there are existing records..."
ast-grep --pattern 'self.env["cx.tower.file.template"].search'

Length of output: 324


Script:

#!/bin/bash
# Search for specific mixin classes and their fields
echo "Searching for specific mixin classes..."
rg -A 10 "class.*(ReferenceMixin|KeyMixin|TemplateMixin).*:"

# Look for model usage patterns
echo -e "\nChecking for model usage patterns..."
rg -l "cx.tower.file.template"

# Check for data files and initialization
echo -e "\nChecking for data files..."
fd -g "data/*.xml"

Length of output: 4273

cetmix_tower_server/views/cx_tower_file_template_view.xml (2)

9-23: LGTM! Archive/Unarchive buttons implementation follows Odoo standards.

The implementation correctly uses:

  • toggle_active method
  • Proper visibility attributes
  • Standard button styling

46-46: LGTM! Invisible active field placement is correct.

The active field is correctly placed in the title div and set as invisible, following Odoo standards.

Copy link
Contributor

@ivs-cetmix ivs-cetmix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Check the comments pls.

  2. Related flight plan lines are not displayed:
    image

@@ -44,7 +44,9 @@ class CxTowerPlanLine(models.Model):
help="Actions trigger based on command result."
" If empty next command will be executed",
)
command_code = fields.Text(related="command_id.code", readonly=True)
command_code = fields.Text(
comodel_name="cx.tower.command", string="Code", readonly=True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason behind removing the "related" key?

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
cetmix_tower_server/models/cx_tower_plan_line.py (3)

60-65: Remove redundant comodel_name attribute.

The comodel_name attribute is unnecessary for related fields as they inherit the model from their related path.

 command_code = fields.Text(
     related="command_id.code",
-    comodel_name="cx.tower.command",
     string="Code",
     readonly=True,
 )

88-108: Consider refactoring compute methods to reduce duplication.

Both compute methods share similar logic patterns. Consider extracting the common logic into a helper method.

+    def _get_related_flight_plan(self):
+        """Helper method to get related flight plan if action is 'plan'."""
+        return (
+            self.command_id.flight_plan_id
+            if self.action == "plan" and self.command_id.flight_plan_id
+            else False
+        )
+
     @api.depends("command_id", "command_id.flight_plan_id", "action")
     def _compute_related_plan_line_ids(self):
         """Compute the related plan lines if the action is "plan"."""
         for line in self:
-            if line.action == "plan" and line.command_id.flight_plan_id:
-                line.related_plan_line_ids = line.command_id.flight_plan_id.line_ids
-            else:
-                line.related_plan_line_ids = False
+            flight_plan = line._get_related_flight_plan()
+            line.related_plan_line_ids = flight_plan and flight_plan.line_ids or False

     @api.depends("command_id", "action")
     def _compute_related_flight_plan_id(self):
         """Compute related Flight Plan ID if the command action is "plan"."""
         for line in self:
-            if line.action == "plan" and line.command_id.flight_plan_id:
-                line.related_flight_plan_id = line.command_id.flight_plan_id
-            else:
-                line.related_flight_plan_id = False
+            line.related_flight_plan_id = line._get_related_flight_plan()

90-92: Fix docstring line length to comply with PEP 8.

The docstrings exceed the recommended line length. Let's make them more concise.

-        """
-        Compute the related plan lines if the action is "plan".
-        """
+        """Compute the related plan lines if the action is "plan"."""

-        """
-        Compute related Flight Plan ID if the command action is "plan".
-        """
+        """Compute related Flight Plan ID if the command action is "plan"."""

Also applies to: 101-103

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed9aaf9 and 54b8da0.

📒 Files selected for processing (2)
  • cetmix_tower_server/models/cx_tower_plan_line.py (3 hunks)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test with OCB
🔇 Additional comments (1)
cetmix_tower_server/models/cx_tower_plan_line.py (1)

28-40: LGTM! Well-structured field declarations.

The field definitions follow best practices with appropriate storage strategies and access controls.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
cetmix_tower_server/models/cx_tower_plan_line.py (1)

95-105: Consider adding error handling in compute methods.

The compute methods could benefit from additional error handling to gracefully handle edge cases.

     @api.depends("command_id", "command_id.flight_plan_id", "action")
     def _compute_related_plan_line_ids(self):
         """
         Compute the related plan lines if the action is "plan".
         """
         for line in self:
+            try:
                 if line.action == "plan" and line.command_id.flight_plan_id:
                     line.related_plan_line_ids = line.command_id.flight_plan_id.line_ids
                 else:
                     line.related_plan_line_ids = False
+            except Exception as e:
+                _logger.error("Error computing related plan lines: %s", str(e))
+                line.related_plan_line_ids = False

Also applies to: 106-116, 117-129

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54b8da0 and bba40d1.

📒 Files selected for processing (2)
  • cetmix_tower_server/models/cx_tower_plan_line.py (3 hunks)
  • cetmix_tower_server/views/cx_tower_plan_line_view.xml (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test with OCB
🔇 Additional comments (5)
cetmix_tower_server/views/cx_tower_plan_line_view.xml (3)

26-29: LGTM! Visibility conditions for use_sudo and path fields.

The visibility conditions ensure these fields are only shown when action is ssh_command, which aligns with their purpose.

Also applies to: 30-33


45-49: LGTM! Field visibility logic.

The visibility condition for command_code is correctly set to show only when action is either ssh_command or python_code.


51-58: LGTM! Related fields visibility.

The visibility conditions for related fields are correctly implemented:

  • related_file_template_id is shown only when action is file_using_template.
  • related_flight_plan_id and related_plan_line_ids are shown only when action is plan.

Also applies to: 59-67

cetmix_tower_server/models/cx_tower_plan_line.py (2)

28-33: LGTM! Field definitions.

The new fields are correctly defined with appropriate attributes:

  • All fields are marked as readonly=True to prevent direct modifications.
  • Compute methods are specified for each field.
  • Appropriate comodel_name values are set.

Also applies to: 34-40, 87-93


60-65: LGTM! Updated command_code field.

The field definition is enhanced with comodel_name and a clearer string label.

cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
cetmix_tower_server/models/cx_tower_plan_line.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ivs-cetmix ivs-cetmix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Command with action "Flight Plan"

  1. Expected behaviour: plan lines are displayed.
  2. Current behaviour:
    image

flight_plan_id = fields.Many2one(
comodel_name="cx.tower.plan",
related="command_id.flight_plan_id",
store=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to store it? Are we searching by this field?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave it in this field we leave it as it can be used in filters/search

file_template_id = fields.Many2one(
comodel_name="cx.tower.file.template",
related="command_id.file_template_id",
store=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to store this? It yes, then for what reason?

@@ -6,6 +6,21 @@
<field name="model">cx.tower.file.template</field>
<field name="arch" type="xml">
<form>
<header>
<button
name="toggle_active"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to add buttons for that. There is a standard option in the "Actions" menu for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants